Skip to content

Conversation

beccadax
Copy link
Contributor

When a C++ type ForeignTy is imported as a foreign reference type, Swift should strip a level of indirection from parameters of that type, so e.g. ForeignTy * and ForeignTy & should be imported to Swift as just ForeignTy. However, it should only strip one level of indirection. importType() was instead stripping all levels, so things like ForeignTy ** and ForeignTy *& were also being incorrectly imported as ForeignTy. Narrow this behavior so it only applies to a single level of indirection and add a few test cases to pin down the specifics.

Fixes rdar://123905345.

When a C++ type `ForeignTy` is imported as a foreign reference type, Swift should strip a level of indirection from parameters of that type, so e.g. `ForeignTy *` and `ForeignTy &` should be imported to Swift as just `ForeignTy`.  However, it should only strip *one* level of indirection. importType() was instead stripping *all* levels, so things like `ForeignTy **` and `ForeignTy *&` were *also* being incorrectly imported as `ForeignTy`. Narrow this behavior so it only applies to a single level of indirection and add a few test cases to pin down the specifics.

Fixes rdar://123905345.
@beccadax beccadax added the c++ interop Feature: Interoperability with C++ label May 22, 2024
@beccadax beccadax marked this pull request as ready for review May 22, 2024 00:35
@beccadax beccadax requested a review from ravikandhadai May 22, 2024 00:35
@beccadax
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@zoecarver zoecarver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thank you!

// CHECK-LABEL: func takeLoggersByConstReference(
// CHECK-SAME: _ pointee0: inout LoggerSingleton!,
// CHECK-SAME: _ pointee1: inout LoggerSingleton!,
// CHECK-SAME: _ pointer: LoggerSingleton!)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems correct to me and matches the way we hide the const& indirection for value types.

Copy link
Contributor

@egorzhdan egorzhdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants